-
Notifications
You must be signed in to change notification settings - Fork 80
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add filter
function as an option to linkinator.check()
#120
feat: add filter
function as an option to linkinator.check()
#120
Conversation
There were the following issues with this Pull Request
You may need to change the commit messages to comply with the repository contributing guidelines. 🤖 This comment was generated by commitlint[bot]. Please report issues here. Happy coding! |
bc60619
to
99c99a1
Compare
filter
function as an option to linkinator.check
filter
function as an option to linkinator.checkfilter
function as an option to linkinator.check
filter
function as an option to linkinator.checkfilter
function as an option to linkinator.check()
Some alternative is making it like in babel/webpack or other tools. Existing flag |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aside from the naming feedback, this is great!
Thanks for the feedback. I've updated this PR to allow |
src/index.ts
Outdated
@@ -16,7 +16,7 @@ export interface CheckOptions { | |||
port?: number; | |||
path: string; | |||
recurse?: boolean; | |||
linksToSkip?: string[]; | |||
linksToSkip?: string[] | ((link: string) => boolean); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to be safe - can we make the function here async
? I never think someone will need it, then they always end up needing it 😆
linksToSkip?: string[] | (async (link: string) => boolean);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So that's weird - is this VS Code? Alternatively, (link: string) => Promise<boolean>
gets us to the same place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah it's VS Code.
I used your Promise<boolean>
suggestion here and updated the tests and docs in a146f2c
There were the following issues with this Pull Request
You may need to change the commit messages to comply with the repository contributing guidelines. 🤖 This comment was generated by commitlint[bot]. Please report issues here. Happy coding! |
f1dfd01
to
a146f2c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great now. Thank you!
🎉 This PR is included in version 1.8.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This PR adds a new
filterLinks
option to thelinkinator.check
function. The new option is a function that can be used to determine which URLs to skip and which to keep.I'm not attached to the name of this option, nor the implementation. I'm open to alternatives ways of solving this that would expose the same level of flexibility to module users.